fix: bugfix three winding transformers in graphs#198
Merged
Conversation
…into fix/three-winding-with-cycle-representation
DCO Remediation Commit for Vincent Koppen <vincent.koppen@alliander.com> I, Vincent Koppen <vincent.koppen@alliander.com>, hereby add my Signed-off-by to this commit: b305453 I, Vincent Koppen <vincent.koppen@alliander.com>, hereby add my Signed-off-by to this commit: 9701f0c I, Vincent Koppen <vincent.koppen@alliander.com>, hereby add my Signed-off-by to this commit: affb900 I, Vincent Koppen <vincent.koppen@alliander.com>, hereby add my Signed-off-by to this commit: ed61f71 Signed-off-by: Vincent Koppen <vincent.koppen@alliander.com>
Signed-off-by: Vincent Koppen <vincent.koppen@alliander.com>
Signed-off-by: Vincent Koppen <vincent.koppen@alliander.com>
|
Member
jaapschoutenalliander
left a comment
There was a problem hiding this comment.
Happy with the suggested solution for now. Some input on the implementation but mainly looks good
Thijss
reviewed
May 11, 2026
Signed-off-by: Vincent Koppen <vincent.koppen@alliander.com>
Co-authored-by: Jaap Schouten <58551444+jaapschoutenalliander@users.noreply.github.com> Signed-off-by: Vincent Koppen <53343926+vincentkoppen@users.noreply.github.com>
Signed-off-by: Vincent Koppen <vincent.koppen@alliander.com>
…h-cycle-representation Signed-off-by: Vincent Koppen <vincent.koppen@alliander.com>
Signed-off-by: Vincent Koppen <vincent.koppen@alliander.com>
jaapschoutenalliander
approved these changes
Jun 3, 2026
…into fix/three-winding-with-cycle-representation Signed-off-by: Vincent Koppen <vincent.koppen@alliander.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Depends on #267 to be merged first
Fixes #167
There were multiple ways to solve this problem:
Disadvantage on memory
Disadvantage: a lot of current algorithms/queries become harder, since you always need to hop / remove the node.
Neither option is perfect, but for now keeping it mostly as is and fixing the two algorithms that were incorrect seems like the easiest/cleanest option.
For these algorithms we:
This way we do not return additional paths that are not there, nor do we see cycles that are not there (since we removed the cycles introduced by the three winding trafo's).
Later if we add even more algorithm we can always revisit again en see whether one of the other option becomes a better solution.